-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not consider DOTNET_ROOT when finding dotnet #69010
Conversation
This changes our `dotnet` launch code to not consider `$DOTNET_ROOT` as that is a apphost specific variable. After discussion with the runtime team determined that only `$PATH` will be considered for finding `dotnet`. dotnet/runtime#88754
Reading the linked issue, I have no idea why not looking in DOTNET_ROOT is the right thing to do. If anything, I draw the opposite conclusion. Before we proceed with this, I think we need the guidance @richlander mentioned clearly documented. |
You can see there that |
@@ -53,15 +53,15 @@ internal static (string processFilePath, string commandLineArguments, string too | |||
/// Get the path to the dotnet executable. In the case the host did not provide this information | |||
/// in the environment this will return simply "dotnet". | |||
/// </summary> | |||
/// <remarks> | |||
/// See the following issue for rationale why only %PATH% is considered | |||
/// https://github.com/dotnet/runtime/issues/88754 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd honestly prefer the docs link here, that was far clearer than this thread 🙂
@dotnet/roslyn-compiler can i get one more review here, small change. |
This changes our
dotnet
launch code to not consider$DOTNET_ROOT
as that is a apphost specific variable.After discussion with the runtime team determined that only
$PATH
will be considered for findingdotnet
.dotnet/runtime#88754